docs(repo): resolve Tier-4 plan-readiness audit findings#124
Merged
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NS-16 — Tier-4 plan-readiness audit of Plan-005 (Provider Driver Contract + Capabilities), Plan-006 (Session Event Taxonomy + Audit Log), and Plan-007 remainder (Local IPC + Daemon Control, Phases 4–7). Swaps the audited working copies into the corpus and lands the cross-cutting contract shapes the audit ratified. Plan-005 + Plan-006 stay
approved; Plan-007 flipsapproved→reviewfor the Tier-4 remainder Phases 4–7 design reopen — the Phases 1–3 partial PR sequence (#16/#17/#19) is audit-evidence preserved in §Tier 1 Partial PR Sequence + §Progress Log + §Retroactive Audit Memo.Audit runbook:
docs/operations/plan-implementation-readiness-audit-runbook.md. This is the Tier-4 pass (Tier-3 = NS-15 merged in PR #118; G6 satisfied).Decisions ratified (D1–D7 + G3 overage)
approved; Plan-007 flipsapproved→reviewper D2-32603 InternalErrorenvelope + reuse oftransport.unavailablecode for daemon-state-degraded refusalsclipanion@4.0.0-rc.4exact-RC pin (Yarn ecosystem precedent + v3 API-incompat avoidance); BL-134 tracks stable-v4 upgrade@napi-rs/keyring v1.2.0ratified (supersedes keytar; native binding sourced from npm preserving pnpmside-effects-cache:falseper Plan-001 PR #3 precedent)security-architecture.md(capability boundary + audit emitter wiring)G5 ephemeral-ref scrub
~100
F-XXX-Y-ZZID prefixes +PR #Ninline refs stripped from audit-introduced plan-body text per Tier-3 Plan-003 canonical precedent (PR #118). Plan-007 baseline-preserved zones (§Tier 1 Partial PR Sequence, §Progress Log, §Retroactive Audit Memo) retain 13PR #Nrefs as legitimate audit-evidence; narrative content preserved verbatim with only ephemeral prefixes scrubbed.Anchor + path corrections (pre-commit-hook-surfaced)
The audit-introducer subagents emitted citation-shape defects the
docs-anchor-checkhook (lychee) caught at commit time. Root-cause fixes applied in this PR (not deferred):ADR-004citation with non-existent filename slug (004-event-streams-are-private-not-shared.md; real ADR-004 = SQLite/Postgres). Replaced with ADR-017 (Shared Event-Sourcing Scope) — the ADR the body'sruntime_node.*driver-event emission actually invokes (line 271). Header + Preconditions + Done-Checklist all updated consistently.004-run-state-machine-and-events.md). Corrected to canonical004-queue-steer-pause-resume.md.../../../../docs/architecture/contracts/api-payload-contracts.md). Corrected to 1-up (../architecture/contracts/api-payload-contracts.md).#idempotency-classesnot in target). Corrected to canonical#idempotency-classes-and-recovery-behavior..agents/tmp/research/...link target ((.)no-op markdown link to transient research path). Sentence rewritten per AGENTS.md Surface-Forward-Then-Delete anti-pattern (the load-bearing content is already surface-forwarded into this plan + theapi-payload-contracts.md§Plan-006 doc-mirror).File manifest
docs/plans/005-provider-driver-contract-and-capabilities.mdidempotency_class+ProviderToolMetadatacross-cutting shapes ratified; ADR-017 added to Required-ADRs (replacing phantom ADR-004); CP-005-5 + CP-005-6 resolutions surface-forwardeddocs/plans/006-session-event-taxonomy-and-audit-log.mdDaemonSigningKeySource+OsKeystoreSealedDaemonSigningKeySource(F-006-2-02 resolution) ratified;retention_classdiscriminator column added (F-006-3-02 Phase 3 Design B);session_snapshotscompaction-cursor columns (F-006-4-01 Phase 4 Reading (a))docs/plans/007-local-ipc-and-daemon-control.mdrun.*/repo.*/artifact.*/settings.*/daemon.*namespace handlers + Spec-027 row 2/3/7a/7b/8 daemon integrations); status flipsapproved→reviewper D2docs/architecture/contracts/api-payload-contracts.mdevent.readAfterCursor,event.readWindow,event.subscribe) per CP-006-4 / CP-007-Ndocs/architecture/security-architecture.mddocs/specs/006-session-event-taxonomy-and-audit-log.mddocs/backlog.mddocs/architecture/cross-plan-dependencies.mdTag on merge:
plan-readiness-audit-tier-4-complete.§6 fold (cross-plan-dependencies.md)
:::ready→:::completed; PRs checkbox[x] tier-4.blocked→readyper the audit-chain edgeNS-16 → NS-17(audit chain remains strictly serialized through NS-21).Findings ledger — 97 findings, 17 critical-root-causes (5.7:1 collapse ratio)
Per-plan breakdown: Plan-005 = 49 findings (5 critical-roots) · Plan-006 = 15 findings (6 critical-roots) · Plan-007 remainder = 33 findings (7 critical-roots, minus overlap = ~17 distinct closures). Healthy finding-instance / root-cause collapse ratio (~5.7:1) — each consuming phase independently surfaced the same upstream gap from its own vantage, not over-flagging (same pattern as Tier-3 NS-15: 31 findings → 4 roots = 7.75:1).
Root-cause critical → disposition:
-32603 InternalError+transport.unavailablereuseclipanionCLI version pin underspecified@napi-rs/keyring v1.2.0security-architecture.mdapproved→reviewDaemonSigningKeySource+ sealed Ed25519 sourceretention_classdiscriminator columnsession_snapshotscompaction-cursor columnsInterventionTypeenum co-locationCapabilityDetailswrapper +providerFailureDetail.agents/tmp/research/plan-readiness-audit/REVIEW.mddecision block)Calibration + gates
B1–B6: B1 avg-critical/phase (Plan-005=1.25; Plan-006=1.5; Plan-007=1.75) in band; B2 total 97 (per-plan: 49/15/33 — Plan-005 over upper band, justified by 4-Phase × audit-density) · B3 Tasks:blocking (Plan-005=1.5:1; Plan-006=4.67:1) in band ✓ · B4 user-review ~60–90 min ✓ · B5 7 substantive advisor calls (range expanded vs Tier-3 baseline; Tier-4 contract-density justified) · B6 1 status-flip (Plan-007; 0–1) ✓
G1–G6: G1 skeleton additions-only ✓ · G2 all 17 root criticals dispositioned (full matrix in REVIEW.md) ✓ · G3 non-Tasks diff Plan-005 2.52×, Plan-006 1.42×, Plan-007 1.53× — Plan-005 + Plan-007 FAIL the literal <1.5× rule; ratified as audit-scope-justified per D-G3 overage (100% audit-domain content, zero redesign) · G4 all Tasks Steps → Spec coverage + Verifies-invariant ✓ · G5 0 ephemeral
F-XXX-Y-ZZ/PR #Nrefs in audited plan bodies (Plan-007 §Tier 1 Partial PR Sequence + §Progress Log + §Retroactive Audit Memo preserve 13 PR refs as audit-evidence per baseline carryover; verified againstplan-readiness-audit-tier-4-baselinetag) ✓ · G6 Tier-3 committed (NS-15 merged in PR #118) ✓Test plan
pnpm install --frozen-lockfileprovisioned the worktree; all pre-commit + commit-msg hooks green (gitleaks: no leaks; lychee: 603 links / 532 OK / 0 errors / 71 excluded; docs-corpus-checks: governance cite-walk; lint-staged: prettier; commitlint: passed).F-XXX-Y-ZZ/PR #Ntags in Plan-005 + Plan-006 + Plan-007 plan-bodies (Plan-007 baseline preserved zones retain 13 PR refs as legitimate audit-evidence per Tier-3 Plan-003 precedent).git show :path | grepconfirms 0 remnants ofADR-004,004-run-state-machine, 4-up traversal,#idempotency-classesstandalone, or transient.agents/tmp/link targets in the audited Plan-005 + Plan-006 surfaces.Codex review resolution
Round 4 —
182ac84Eleven findings (5×P1) resolved in
182ac84; all eleven review threads replied + resolved. Headline P1: F9 —daemon.startwas modeled as an in-daemon IPC handler, which is impossible (a stopped daemon hosts no IPC server to receive the call). RemovedDaemonStart*from the daemon-side surface (lifecycle schemas, IPC handlers, Tier-4 method-name table, client SDK list, renderer mutating-op gate, and theaction:"start"union + sharedDaemonLifecycleParamsinapi-payload-contracts.md); cold-boot is now theai-sidekicks daemon startCLI process-spawn path (runtime-resolved daemon path + detached spawn +DaemonHelloAckwait, T-007r-3-4). This re-aligns Plan-007 to Spec-023:76's "spawned child process of the shell" framing — the plan had drifted from the spec, not the reverse. Other P1s: F11 (DaemonKeyStore real impl + composition-root injection at Plan-022 Tier 5 per CP-007-8, not R2 — no tier inversion), F1 (R1 registers onlydaemon.*+settings.*; the other five namespaces attach from their owning plans), F10 (banner read is daemon-availability-tolerant), F2 (evidence_pr: 124).Round 5 —
34ee7f7Seven findings (1×P1) resolved in
34ee7f7; all seven threads replied + resolved — 45/45 threads resolved,mergeStateStatus: CLEAN. Headline P1 R5-6: the round-2stub_signaturesigned only the compacted-rowpayloadprojection, leaving the denormalized scalar columns (id,session_id,sequence,occurred_at,category,type,actor) — read by SQL filters and envelope reconstruction — unbound; an at-rest scalar edit could forge anactor/typewhile bothstub_signatureand the anchor over the frozenrow_hashstill verified. The verifier now adds a scalar-binding check (each scalar column byte-equals itspayload-projection counterpart) → a newfailureMode: 'stub_scalar_mismatch', an additive-MINOR enum extension 10→11 per ADR-018 §Decision #8. Other findings: R5-1 (DaemonStatusReadResultAPI mirror reconciled to the canonicalprocessStateshape), R5-2 (Spec-007 amended to disambiguate daemonstartas a process-spawn capability, not adaemon.startIPC handler), R5-3 (separateDaemonStopRequest/DaemonRestartRequestmirror schemas carryingidleDrainDeadlineMs), R5-4 (Plan-005DriverClientinterface enumerates exactly the 7 client-facing methods, not the stale 11), R5-5 (Plan-005 storage summary lists all four Phase-2 SQLite tables), R5-7 (R2-T6 re-scoped to the emission-sidesecurityCriticalboundary R2 owns).Round-4 deferrals, now resolved. The two round-4 "named but not chased" divergences are closed: (a) the
api-payload-contracts.mdDaemonStatusReadResultmirror is reconciled to the canonicalprocessStateshape (R5-1); (b) the Plan-007 CLI §Files inventory (one-vs-threedaemon-lifecycle.ts) is fixed in8631a1d, which also reconciled the broader R3 §Files block to its authoritative Tasks — addedexit-codes.ts(T-3-3) +settings.ts(T-3-6), fixed thedaemon-status.tscitation (T-3-3→T-3-5), and removed thesession-*/meta.tsorphans (the CLIsessionsurface is owned by Plan-001 Phase 5;--version/--helpare clipanionCli-builder built-ins).docs/specs/007-local-ipc-and-daemon-control.md:74is now amended (R5-2) — a real defect in the approved spec's IPC-method listing, not an intentional carry-forward.Remaining deferrals (each owned by its own pending audit).
DaemonStartsurvives in the capability sense (the shell boots the daemon, realized by spawn) in three Tier-8 peer artifacts revisited by their own audits:docs/specs/023-desktop-shell-and-renderer.md:76,221,docs/plans/023-desktop-shell-and-renderer.md:62,docs/operations/local-daemon-runbook.md. Amending a peer plan/spec/runbook to restate something already correct in the capability sense is out of this PR's scope.Refs: Plan-005, Plan-006, Plan-007, Spec-006, Spec-007, ADR-017, ADR-018, BL-134
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com